Repository navigation
Conversation
Add DebugLayoutReadoutState and internal DebugLayoutReadout composable to display window container size in dp, display rotation degrees, rotation-lock status, and active adaptive layout mode in DebugConsole, falling back to live LocalWindowInfo container dimensions when explicit posture state is omitted. Also fix DebugTextBar applying its outer Row modifier three times to its child Text nodes, configure Robolectric Compose unit testing for :ui:debug, and add unit tests covering explicit/default readouts, live window fallback, modifier scoping, and hidden-component behavior.
…yout-readout # Conflicts: # ui/debug/src/main/java/com/google/jetpackcamera/ui/debug/DebugOverlayComponents.kt
…80-degree gravity delta
…/debug-layout-readout # Conflicts: # feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewScreen.kt # ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/CaptureLayout.kt
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces an adaptive portrait layout solver and updates the capture screen UI to support adaptive layout and rotation behaviors across various device form factors, alongside a new microbenchmark module to measure layout solver performance. The review feedback highlights a critical compilation error in PreviewScreen.kt regarding the usage of repeatOnLifecycle, and suggests a more robust recursive unwrapping method to safely retrieve the Activity from the context. Additionally, the feedback points out that two new test classes violate the repository style guide by omitting the @RunWith annotation.
| LaunchedEffect(lifecycleOwner, context) { | ||
| lifecycleOwner.repeatOnLifecycle(Lifecycle.State.STARTED) { | ||
| debouncedOrientationFlow(context, initialRotation = windowRotation).collect { |
There was a problem hiding this comment.
The repeatOnLifecycle extension function is defined on Lifecycle, not LifecycleOwner. Calling it directly on lifecycleOwner will cause a compilation error. It should be called on lifecycleOwner.lifecycle instead.
| LaunchedEffect(lifecycleOwner, context) { | |
| lifecycleOwner.repeatOnLifecycle(Lifecycle.State.STARTED) { | |
| debouncedOrientationFlow(context, initialRotation = windowRotation).collect { | |
| LaunchedEffect(lifecycleOwner, context) { | |
| lifecycleOwner.lifecycle.repeatOnLifecycle(Lifecycle.State.STARTED) { | |
| debouncedOrientationFlow(context, initialRotation = windowRotation).collect { |
| } | ||
| } | ||
|
|
||
| val activity = context as? Activity |
There was a problem hiding this comment.
Casting context as? Activity directly can fail if the context is wrapped in a ContextWrapper (such as Hilt's ViewComponentManager.FragmentContextWrapper or a ContextThemeWrapper). To ensure the activity is correctly resolved and the orientation can be locked during video recording, recursively unwrap the context to find the underlying Activity.
val activity = remember(context) {
var c = context
while (c is android.content.ContextWrapper) {
if (c is Activity) return@remember c
c = c.baseContext
}
c as? Activity
}| import org.junit.Test | ||
|
|
||
| /** | ||
| * Unit tests for [resolveRequestedOrientation]. | ||
| */ | ||
| class MainActivityOrientationTest { |
There was a problem hiding this comment.
According to the repository style guide, all test classes must be annotated with @RunWith(...) to explicitly declare which test runner should be used. Since this test uses Android-specific classes like ActivityInfo, it should be run with RobolectricTestRunner.
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner
/**
* Unit tests for [resolveRequestedOrientation].
*/
@RunWith(RobolectricTestRunner::class)
class MainActivityOrientationTest {| import android.view.Surface | ||
| import com.google.common.truth.Truth.assertThat | ||
| import org.junit.Test | ||
|
|
||
| class DeviceRotationTest { |
There was a problem hiding this comment.
According to the repository style guide, all test classes must be annotated with @RunWith(...) to explicitly declare which test runner should be used. Since this test uses Android-specific classes like Surface, it should be run with RobolectricTestRunner.
import android.view.Surface
import com.google.common.truth.Truth.assertThat
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner
@RunWith(RobolectricTestRunner::class)
class DeviceRotationTest {…/debug-layout-readout # Conflicts: # core/model/src/main/java/com/google/jetpackcamera/model/DeviceRotation.kt # feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewScreen.kt # ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/DebouncedOrientationFlow.kt
…on, active layout readout, and recording orientation lock
…/debug-layout-readout
… and prune dead helper Co-authored-by: davidjiagoogle <[email protected]>
Kimblebee
changed the title
…snap 180-degree flips
Summary
Now that
kim/adaptive/preview-display-framingunlocksMainActivityorientation on large screens and multi-window panes, physical hardware gravity (deviceRotation) and OS display posture (windowRotation) can vary independently. This PR introduces the 3-signal rotation foundation (deviceRotation,windowRotation, and in-windowgravityRotationDegrees), draw- and layout-phase rotation modifiers (animateToUpright,rotateLayoutToGravity), mid-recording orientation locking inCameraSessionEffect, andDebugLayoutReadoutin:ui:debug(Layout,Mode,Form Factor,Device Rotation,Window Rotation,Expected Layout Rotation, andIcon Rotation).Acknowledgement: Adapts
DeviceRotationorientation helpers,RotationModifiers.kt(rememberWindowRotationdisplay listener, shortest-path upright animation,animateToUpright, and constraint-swapping layout rotation), and the video-recording orientation lock from PR #619, generalizing them from a fixed-portrait frame into the 3-signal relative model (deviceRotation,windowRotation, gravity delta).Key Changes
DebugOverlayComponents.kt,TestTags.kt,strings.xml,ui/debug/build.gradle.kts(:ui:debug):DebugWindowOrientation, 7-mode hysteresis-stabilizedDebugFormFactor(±7dpdeadbands),DebugLayoutReadoutState,DebugLayoutReadout, and an animated gravity-pointingDebugUpwardTriangleIndicatortoDebugConsole, with individual test tags for each line. Uses liveLocalWindowInfo.current.containerSizedimensions by default when explicit state is omitted, and hides whendebugHidingComponentsis enabled.:ui:debug.DeviceRotation.kt(:core:model):DeviceRotation.toClockwiseRotationDegrees(),DeviceRotation.toGravityRotationDegrees(windowRotation)(clamping180°deltas to0°),toWindowCounterRotationDegrees(), andisLandscape.RotationModifiers.kt,DebouncedOrientationFlow.kt,CaptureScaffold.kt,CaptureLayout.kt,PreviewScreen.kt(:ui:components:capture,:feature:preview):LocalDeviceRotation,LocalWindowRotation,LocalGravityRotationState,rememberWindowRotation()(LocalConfiguration+DisplayManager.DisplayListenerto catch seamless180°landscape flips),rememberDeviceRotation(),rememberGravityRotationState()(shortest-path250mstween on90°quarter-turns by default viaanimateQuarterTurns = true; instantsnap()on180°flips, window-rotation changes, or whenanimateQuarterTurns = false), draw-phaseModifier.animateToUpright(), and layout-phaseModifier.rotateLayoutToGravity().deviceRotationandwindowRotationthroughPreviewScreen->CaptureScaffold->CaptureScaffoldLayout, and enforces symmetric±5°hysteresis around candidate quadrant centers indebouncedOrientationFlow.CameraSessionEffect.kt,PreviewScreen.kt(Recording Orientation Lock):videoRecordingState !is VideoRecordingState.Inactive), locks the hostActivitytoSCREEN_ORIENTATION_LOCKEDand suppresses live sensor rotation updates, restoring the previousrequestedOrientationand flushing any buffered sensor rotation when recording ends.How Rotation & Posture Math Works (and Resolves Single-Signal Limitations)
Phone-only landscape logic derives UI rotation from
deviceRotationalone (assuming the Activity window is permanently locked to0°). Once the window itself can rotate (on foldables, tablets, or in multi-window split-screen):90°, rotating icons bydeviceRotation(90°) double-rotates them sideways relative to the already-rotated window. Conversely, if a split-screen window is rotated90°while the user holds the device at0°,deviceRotation(0°) fails to counter-rotate the icons.toGravityRotationDegrees):DeviceRotation.fromUiSurfaceRotationmapsSurface.ROTATION_90 -> Rotated270andSurface.ROTATION_270 -> Rotated90sowindowRotationsharesdeviceRotation's physical clockwise frame.toGravityRotationDegreesthen computes((windowDeg - deviceDeg) % 360 + 360) % 360so icons only counter-rotate by the residual delta between the window and physical gravity (clamping180°to0°so icons and layouts do not flip upside-down, while CameraX still receivesDeviceRotation.Rotated180for capture metadata).Modifier.animateToUpright()readsState<Float>.valuestrictly insidegraphicsLayer(RenderNode-only invalidation, zero recomposition/re-layout).Modifier.rotateLayoutToGravity()swaps boundedConstraintsaxes and outer(width, height)at90°/270°and centers viaplaceWithLayerso rectangular surfaces neither clip nor misalign hit targets.rememberGravityRotationStateanimates90°quarter-turns over250msby default (animateQuarterTurns = true) while snapping immediately on180°half-turn flips (90° <-> 270°) and wheneverwindowRotationchanges so icons do not animate inside an already-rotating OS window.Form Factor & Expected Layout Rotation Table (
DebugFormFactor.fromWindowDp)Let
minDim = min(widthDp, heightDp),maxDim = max(widthDp, heightDp), andratio = maxDim / minDim(with±7dphysteresis aroundpreviouswhen dragged across a boundary):fromWindowDp)H >= W) Expected Layout RotationW > H) Expected Layout RotationUNKNOWNwidthDp <= 0 || heightDp <= 00°0°COMPACT_SHORT_SLIMwidthDp < 480dp && heightDp < 480dp && minDim < 360dp0°(condensed side rails)0°(condensed side rails)COMPACT_SHORTwidthDp < 480dp && heightDp < 480dp(minDim >= 360dp)0°(condensed side rails)0°(condensed side rails)COMPACT_SLIMminDim < 360dp(andmaxDim >= 480dp)0°90°or270°COMPACTwidthDp < 600dp || heightDp < 480dp(minDim >= 360dp,maxDim >= 480dp)0°90°or270°FOLDABLEwidthDp >= 600dp && heightDp >= 480dp && maxDim < 960dp && ratio < 1.45f0°(upright; side rails adapt)0°(upright; side rails adapt)TABLET_PORTRAITwidthDp >= 600dp && heightDp >= 480dp(non-foldable) andheightDp >= widthDp0°(upright; side rails adapt)TABLET_LANDSCAPEwidthDp >= 600dp && heightDp >= 480dp(non-foldable) andwidthDp > heightDp0°(upright; side rails adapt)(For wide
COMPACT/COMPACT_SLIMpostures,expectedLayoutRotationDegreesusesiconRotationDegreeswhen90°/270°, falls back to the window counter-rotation whenwindowRotationis90°/270°, and defaults to270°so controls sit on the right edge.)Bugs Fixed Along the Way
DebugTextBarduplicate modifier application (DebugOverlayComponents.kt):DebugTextBarpreviously passed itsmodifierparameter to the outerRowand again to both childTextviews, applying any padding, sizing, ortestTagthree times.180°) quadrant boundary jitter (DebouncedOrientationFlow.kt): Checking distance tocandidatecenter (<= 40°) rather than distance fromprevSnap(>= 50°) ensures the±5°deadband holds across all four quadrant boundaries.CameraSessionEffect.kt): Prevents sensorsetDisplayRotationupdates and OS window orientation changes from firing mid-recording, while guaranteeing stale orientation is flushed once recording stops.Not Included Yet
.counterRotateToPortrait, inset/cutout remapping, and liveCameraLayoutModereadout wiring (kim/adaptive/layout-rotation).Preview.targetRotationandViewPortsynchronization withwindowRotation(kim/adaptive/viewfinder-rotation)..animateToUpright()/.rotateLayoutToGravity()adoption and sticky180°gravity retention (kim/adaptive/content-gravity).Testing
:core:model:testDebugUnitTest—DeviceRotationTest(all 16deviceRotation × windowRotationcombinations,Surfacerotation round-trip,180°clamp, counter-rotation angles).:ui:components:capture:testDebugUnitTest—RotationModifiersTest(shortest-path0° <-> 270°unwrapping,animateQuarterTurns = truedefault tween with180°flip snap,animateQuarterTurns = falseimmediate snap,animateToUpright,rotateLayoutToGravity90°/270°dimension swap) andCaptureScaffoldTest(CameraSessionEffectrecording orientation lock and restore).:ui:debug:testDebugUnitTest—DebugLayoutReadoutTest(7-modeDebugFormFactor.fromWindowDpclassification and±7dphysteresis,expectedLayoutRotationDegreesincluding550x500dpwideCOMPACTvs.<480dpCOMPACT_SHORT, per-field test tags, single-nodeDebugTextBarmodifier scoping, anddebugHidingComponentshiding).